-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try: Add new textAlign
block support
#59531
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +434 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
@aaronrobertshaw Thank you for your additional feedback 🙏
I was able to reproduce this issue using the theme.json below: {
"$schema": "https://raw.githubusercontent.com/WordPress/gutenberg/add/text-align-block-support/schemas/json/theme.json",
"version": 2,
"styles": {
"blocks": {
"core/media-text": {
"typography": {
"textAlign": "center"
}
}
}
}
} After updating
Regarding the block level, there is currently no design feedback, but I hope it will be displayed as a list of buttons, similar to Decoration and Letter Case. Also, this PR initially attempted to support Instead, it is proposed to support Do you think adding textAlign UI to the Global Styles should be considered in this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this PR is ready to go @t-hamano, nice work 🚢
After the latest update, theme.json based styling for text align is working correctly in the site editor for me. A quick smoke test again on the other aspects of the block support didn't show any issues.
Do you think adding textAlign UI to the Global Styles should be considered in this PR?
I don't think the lack of Global Styles support should be considered a blocker for this PR.
It can be a follow-up but probably should be added before text align support is offered via WP6.6. There is some time yet before the beta so it should be fine.
Thank you everyone for your reviews! I think this PR is ready to ship, so I would like to merge it.
I see, I would like to continue working on the following tasks (please let me know if I have misunderstood anything).
|
Sounds like a plan @t-hamano 👍 My vote would be to lean into sorting out the Global Styles UI first, then update the core blocks to adopt the support after that. |
@t-hamano Does this need to be backported to 6.6? |
Thanks! It's fine to wait for more GB PRs, but I think generally we can already create backport PRs / Trac tickets and add to them? |
Ready to backport to the core. |
See WordPress/gutenberg#59531. See WordPress/gutenberg#61182. See WordPress/gutenberg#61717. See #6590. Fixes #61256. Props wildworks, ellatrix. git-svn-id: https://develop.svn.wordpress.org/trunk@58314 602fd350-edb4-49c9-b593-d223f7449a82
See WordPress/gutenberg#59531. See WordPress/gutenberg#61182. See WordPress/gutenberg#61717. See WordPress/wordpress-develop#6590. Fixes #61256. Props wildworks, ellatrix. Built from https://develop.svn.wordpress.org/trunk@58314 git-svn-id: https://core.svn.wordpress.org/trunk@57771 1a063a9b-81f0-0310-95a4-ce76da25c4cd
See WordPress/gutenberg#59531. See WordPress/gutenberg#61182. See WordPress/gutenberg#61717. See WordPress/wordpress-develop#6590. Fixes #61256. Props wildworks, ellatrix. Built from https://develop.svn.wordpress.org/trunk@58314 git-svn-id: http://core.svn.wordpress.org/trunk@57771 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Fixes #38427
Related to #8450, #48202
What?
This PR attempts to add a new
typography.textAlign
(left/center/right/justify) block support. I only have a basic implementation at the moment, but I'd love to hear your thoughts on whether this approach makes sense!Why?
The current text-align setting is implemented ad hoc in each block. In other words, the code looks like this:
Code like this exists in over 20 core blocks, so I think it makes sense to unify the block support.
I also think there are two benefits for developers:
How?
The implementation itself is very similar to
align
support, with the following characteristics:boolean | array
can be defined as a value.true
, allleft/center/right/~~justify~~
options can be controlled.array
, only the options that match the array elements can be controlled.default
, the toolbar control will not be available.Additionally, if we replace this support in the core blocks in the future, the markup saved will remain the same, so there should be no need to add a deprecation.Update: For all blocks, if text-align changes, the class namehas-text-align-{align}
is added. This PR, on the other hand, is output as an inline style. Therefore, we will need to add deprecations in every block.However, I think we should note the following points:
The only exception is the Paragraph block, which is defined as an attribute calledalign
instead oftextAlign
. Therefore, it will be necessary to add deprecation only to this block, and adjust the alignment inheritance when transforming from/to the heading block.Testing Instructions
Block Support
This PR purely adds block support only and has no impact on core blocks. Use the code below to register a block that supports
textAlign
via the browser console and check the operation of the block toolbar.Code
Additionally, to ensure that core blocks can migrate to this support in the future, we will temporarily make the following changes to ensure that they work with a variety of block types.
Static Block (Media & Text Block)
It should work as expected, both on the editor and on the frontend.
Dynamic Block (Archives Block)
It should work as expected, both on the editor and on the frontend.
Dynamic Block with __experimentalSkipSerialization (Search Block)
This block skips serializing typography support and gives each label, input field, and button a typography-related class name. On the editor side, the
text-align-{textAlign}
class should not be output to the block wrapper div, but instead the class should be output for each label, input field, and button.Pasting Styles
text-align
setting toright
.text-align
setting (right
) should be applied.Screenshots or screencast
The video below shows that a block with temporary
textAlign
support is registered via the browser console andtextAlign
support works correctly.8c71ff29436ffdf74d9edb4556bde32f.mp4
ToDo
If it makes sense to move forward with this PR, I would like to work on the following tasks.